Skip to content

improve logic of heap_type validation when ref.null #4372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 12 commits into from
Jul 10, 2025
Merged

improve logic of heap_type validation when ref.null #4372

merged 12 commits into from
Jul 10, 2025

Conversation

Septa2112
Copy link
Contributor

PR #4300 introduced the rationale for validating heap_type.

This patch moves the validation before the computation of type1 to prevent potential overflow.

@Septa2112
Follow-up to PR bytecodealliance#4300: prevent potential overflow
fd5cfe4 
PR bytecodealliance#4300 introduced the rationale for validating heap_type.
This patch moves the validation before the computation of
type1 to prevent potential overflow.
@Septa2112 Septa2112 mentioned this pull request Jun 19, 2025
Open
@Septa2112
Copy link
Contributor Author

Further improved the heap type check logic:

  1. Moved the heap type validation ahead of the if (!is_byte_a_type...) check. Depending on whether heap_type >= 0, different methods are used to get type1.

  2. The type_index check is now performed only when heap_type >= 0, to ensure that type1 is correctly retrieved in this case.

  3. Why not checking type_index when heap_type < 0? Because in the current WAMR implementation, if wasm_is_valid_heap_type(heap_type) is true, then the condition if (!is_byte_a_type(type1) || wasm_is_type_multi_byte_type(type1)) will always be false.

lum1n0us
lum1n0us reviewed Jun 19, 2025
View reviewed changes
lum1n0us
lum1n0us reviewed Jun 24, 2025
View reviewed changes
@Septa2112 Septa2112 changed the title Prevent potential overflow improve logic of heap_type validation when ref.null Jun 25, 2025
lum1n0us
lum1n0us reviewed Jun 27, 2025
View reviewed changes
lum1n0us
lum1n0us requested changes Jul 1, 2025
View reviewed changes
@lum1n0us lum1n0us merged commit 903a5c1 into bytecodealliance:main Jul 10, 2025
515 of 520 checks passed
@Septa2112 Septa2112 deleted the fix/overflow branch July 10, 2025 02:15
@lum1n0us lum1n0us added the bug-fix Determine if this PR addresses a bug. It will be used by scripts to classify PRs. label Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lum1n0us lum1n0us lum1n0us approved these changes

@TianlongLiang TianlongLiang TianlongLiang approved these changes

@loganek loganek Awaiting requested review from loganek loganek is a code owner

@no1wudi no1wudi Awaiting requested review from no1wudi no1wudi is a code owner

@wenyongh wenyongh Awaiting requested review from wenyongh wenyongh is a code owner

@xujuntwt95329 xujuntwt95329 Awaiting requested review from xujuntwt95329 xujuntwt95329 is a code owner

@yamt yamt Awaiting requested review from yamt yamt is a code owner

Assignees
No one assigned
Labels
bug-fix Determine if this PR addresses a bug. It will be used by scripts to classify PRs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Septa2112 @lum1n0us @TianlongLiang